Skip to content

IGNITE-28944 Refactor SecurityBasicPermissionSet - #13424

Open
shishkovilja wants to merge 4 commits into
apache:masterfrom
shishkovilja:ignite-28944
Open

IGNITE-28944 Refactor SecurityBasicPermissionSet#13424
shishkovilja wants to merge 4 commits into
apache:masterfrom
shishkovilja:ignite-28944

Conversation

@shishkovilja

Copy link
Copy Markdown
Contributor

No description provided.

@ignitetcbot

Copy link
Copy Markdown
Contributor

TCBot Test Analysis

Possible Blockers (0)

No blockers found.

New Tests (0)

No new tests found.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Possible compatibility issues. Please, check rolling upgrade cases

This PR modifies protected classes (with Order annotation).
Changes to these classes can break rolling upgrade compatibility.

Affected files:

  • modules/core/src/main/java/org/apache/ignite/plugin/security/SecurityBasicPermissionSet.java

}

/** @param map Map. */
@SuppressWarnings("rawtypes")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@SuppressWarnings("rawtypes")
@SuppressWarnings({"rawtypes", "unchecked"})

}

/** @param map Map. */
@SuppressWarnings("rawtypes")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@SuppressWarnings("rawtypes")
@SuppressWarnings({"rawtypes", "unchecked"})

}

/** */
private void convert() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convert -> normalize?

* @param cachePermissions Cache permissions.
* @return Map with enum sets of security permissions.
*/
public static Map<String, Collection<SecurityPermission>> toEnumSetMap(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toEnumSetMap -> normalizeValueType?


/** */
private static EnumSet<SecurityPermission> copySafe(Collection<SecurityPermission> col) {
return col != null ? EnumSet.copyOf(col) : EnumSet.noneOf(SecurityPermission.class);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can check if col is instanceof EnumSet<SecurityPermission>

return upcast(srvcPerms);
}

/** @param map Map. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need something more meaningful here, or we should simply leave it empty.

The same below.


/** {@inheritDoc} */
@Override public void unmarshal(Marshaller marsh, ClassLoader clsLdr) throws IgniteCheckedException {
// Message framework uses ArrayList for ordinary collectons, so we need convert it explicitly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo.

*/
static Collection<SecurityPermission> permissions(SecurityPermission... perms) {
Collection<SecurityPermission> col = U.newHashSet(perms.length);
static EnumSet<SecurityPermission> permissions(SecurityPermission... perms) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we can drop this method and use EnumSet.of(<permisisons>) instead.


/** */
private static void authorizeAll(IgniteSecurity security, Map<String, Collection<SecurityPermission>> permissions) {
private static void authorizeAll(IgniteSecurity security,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No line break is needed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants